Skip to content

[sanitizer_common] Drop support for Android 5 #145227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 8, 2025

Conversation

brad0
Copy link
Contributor

@brad0 brad0 commented Jun 22, 2025

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Jun 22, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Brad Smith (brad0)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/145227.diff

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp (+3-30)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
index 3bf9f8b316d36..653872c9848a5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp
@@ -782,42 +782,15 @@ static int dl_iterate_phdr_cb(dl_phdr_info *info, size_t size, void *arg) {
   return 0;
 }
 
-static bool requiresProcmaps() {
-#  if SANITIZER_ANDROID && __ANDROID_API__ <= 22
-  // Fall back to /proc/maps if dl_iterate_phdr is unavailable or broken.
-  // The runtime check allows the same library to work with
-  // both K and L (and future) Android releases.
-  return AndroidGetApiLevel() <= ANDROID_LOLLIPOP_MR1;
-#  else
-  return false;
-#  endif
-}
-
-static void procmapsInit(InternalMmapVectorNoCtor<LoadedModule> *modules) {
-  MemoryMappingLayout memory_mapping(/*cache_enabled*/ true);
-  memory_mapping.DumpListOfModules(modules);
-}
-
 void ListOfModules::init() {
   clearOrInit();
-  if (requiresProcmaps()) {
-    procmapsInit(&modules_);
-  } else {
-    DlIteratePhdrData data = {&modules_, true};
-    dl_iterate_phdr(dl_iterate_phdr_cb, &data);
-  }
+  DlIteratePhdrData data = {&modules_, true};
+  dl_iterate_phdr(dl_iterate_phdr_cb, &data);
 }
 
 // When a custom loader is used, dl_iterate_phdr may not contain the full
 // list of modules. Allow callers to fall back to using procmaps.
-void ListOfModules::fallbackInit() {
-  if (!requiresProcmaps()) {
-    clearOrInit();
-    procmapsInit(&modules_);
-  } else {
-    clear();
-  }
-}
+void ListOfModules::fallbackInit() { clear(); }
 
 // getrusage does not give us the current RSS, only the max RSS.
 // Still, this is better than nothing if /proc/self/statm is not available

@brad0 brad0 requested a review from enh-google June 22, 2025 10:53
@brad0 brad0 force-pushed the sanitizer_android_5 branch from 6cfceb3 to c5054ac Compare June 22, 2025 10:57
@enh-google
Copy link
Contributor

@emrekultursay @DanAlbert fyi

(it probably doesn't matter much in practice -- not least because devices this old are likely to be 32-bit, and arm32 debugging is pretty comprehensively broken already -- but note that this is slightly ahead of us... we're still working on moving to api 22 [or, more likely 23] as our minimium.)

@brad0
Copy link
Contributor Author

brad0 commented Jun 23, 2025

@emrekultursay @DanAlbert fyi

(it probably doesn't matter much in practice -- not least because devices this old are likely to be 32-bit, and arm32 debugging is pretty comprehensively broken already -- but note that this is slightly ahead of us... we're still working on moving to api 22 [or, more likely 23] as our minimium.)

What is the usual path for removing support altogether? As far as I can find the Play Store and Play Services have not supported 5.x for a year now.

@enh-google
Copy link
Contributor

@emrekultursay @DanAlbert fyi
(it probably doesn't matter much in practice -- not least because devices this old are likely to be 32-bit, and arm32 debugging is pretty comprehensively broken already -- but note that this is slightly ahead of us... we're still working on moving to api 22 [or, more likely 23] as our minimium.)

What is the usual path for removing support altogether? As far as I can find the Play Store and Play Services have not supported 5.x for a year now.

i think you mean "a few months" ... it changed from 21 to 23 in February of this year :-)

https://developers.google.com/admob/android/migration#the_minimum_android_api_level_is_23

unfortunately the NDK hasn't caught up yet (that's why i tagged danalbert) and i actually don't know what the Studio policy on such things is (which is why i tagged emrekultursay).

@brad0
Copy link
Contributor Author

brad0 commented Jun 23, 2025

i think you mean "a few months" ... it changed from 21 to 23 in February of this year :-)

https://developers.google.com/admob/android/migration#the_minimum_android_api_level_is_23

unfortunately the NDK hasn't caught up yet (that's why i tagged danalbert) and i actually don't know what the Studio policy on such things is (which is why i tagged emrekultursay).

Everything I came across, so far, said March 2024 (Play Store) and July 2024 (Play Services).

@enh-google
Copy link
Contributor

i think you mean "a few months" ... it changed from 21 to 23 in February of this year :-)
https://developers.google.com/admob/android/migration#the_minimum_android_api_level_is_23
unfortunately the NDK hasn't caught up yet (that's why i tagged danalbert) and i actually don't know what the Studio policy on such things is (which is why i tagged emrekultursay).

Everything I came across, so far, said March 2024 (Play Store) and July 2024 (Play Services).

have you been listening to stochastic parrots by any chance?

https://developers.google.com/admob/android/rel-notes#24.0.0 seems quite clear that v24 was new this february (and seems likely to have been written by a human).

@brad0
Copy link
Contributor Author

brad0 commented Jun 27, 2025

have you been listening to stochastic parrots by any chance?

Partially, although I came across a few other hits that alluded to last year.

https://developers.google.com/admob/android/rel-notes#24.0.0 seems quite clear that v24 was new this february (and seems likely to have been written by a human).

Thanks. Official documentation is always useful. As long as it is up to date (which we went over in the past).

So, being that things only happened this year, I don't mind necessarily letting this wait. Since I was under the impression that it had been a year, not only a number months. It depends on what the feedback is.

@brad0
Copy link
Contributor Author

brad0 commented Jul 2, 2025

cc @emrekultursay @DanAlbert

@fmayer
Copy link
Contributor

fmayer commented Jul 2, 2025

Can you add something to the commit message whether this resolves any action problem, or is just cleanup?

@brad0
Copy link
Contributor Author

brad0 commented Jul 2, 2025

Can you add something to the commit message whether this resolves any action problem, or is just cleanup?

Just cleaning up.

@brad0 brad0 merged commit fe56f69 into llvm:main Jul 8, 2025
7 checks passed
@brad0 brad0 deleted the sanitizer_android_5 branch July 8, 2025 01:06
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jul 8, 2025
@DanAlbert
Copy link
Member

API 21 is still supported and the NDK actively tests this configuration. I have no plans to drop API 21 support until after the next LTS (because I don't like removing features in LTS releases), which probably won't be until next year. This should be reverted.

@enh-google
Copy link
Contributor

enh-google commented Jul 8, 2025 via email

@DanAlbert
Copy link
Member

DanAlbert commented Jul 8, 2025

This is in sanitizer_common. It applies to UBSan too, right?

@enh-google
Copy link
Contributor

This is in sanitizer_common. It applies to UBSan too, right?

in theory i think you can set the environment variables to exercise this, but in practice on Android you're getting a tombstone instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants